Conversation
There was a problem hiding this comment.
Pull request overview
This pull request integrates Cloudflare Turnstile bot protection into the backend by adding turnstile verification to the user invitation endpoint. The implementation includes a new TurnstileService for verifying captcha tokens, a new global SharedModule, and modifications to the company invitation flow to require and validate turnstile tokens.
Changes:
- Added
TurnstileServiceto verify Cloudflare Turnstile tokens via external API - Created a new global
SharedModuleto makeTurnstileServiceavailable across the application - Modified the user invitation endpoint to require and verify a turnstile token before processing invitations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/shared/shared.module.ts | Creates a global module that exports TurnstileService |
| backend/src/shared/services/turnstile.service.ts | Implements the Turnstile verification service with Cloudflare API integration |
| backend/src/entities/company-info/company-info.controller.ts | Adds TurnstileService injection and verification call to the invite user endpoint |
| backend/src/entities/company-info/application/dto/invite-user-in-company-and-connection-group.dto.ts | Adds optional turnstileToken field to the DTO |
| backend/src/app.module.ts | Imports the new SharedModule |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const secretKey = process.env.TURNSTILE_SECRET_KEY; | ||
|
|
||
| if (!secretKey) { | ||
| console.warn('Turnstile secret key is not configured. Skipping verification.'); |
There was a problem hiding this comment.
Using console.warn is inconsistent with the project's logging approach. The codebase uses a WinstonLogger service for logging (as seen in other services and controllers). Consider injecting WinstonLogger and using it here instead of console.warn.
| const response = await axios.post<TurnstileVerifyResponse>(this.verifyUrl, formData.toString(), { | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| }); | ||
|
|
||
| if (!response.data.success) { | ||
| const errorCodes = response.data['error-codes']?.join(', ') || 'Unknown error'; | ||
| throw new BadRequestException(`Turnstile verification failed: ${errorCodes}`); | ||
| } | ||
|
|
||
| return true; |
There was a problem hiding this comment.
Missing error handling for network failures or Cloudflare API errors. If the axios.post call throws an exception (network timeout, DNS failure, etc.), it will propagate unhandled. Other axios calls in the codebase use try-catch with proper error handling (see table-action-activation.service.ts). Wrap this call in a try-catch block and provide a meaningful error message.
| const response = await axios.post<TurnstileVerifyResponse>(this.verifyUrl, formData.toString(), { | |
| headers: { | |
| 'Content-Type': 'application/x-www-form-urlencoded', | |
| }, | |
| }); | |
| if (!response.data.success) { | |
| const errorCodes = response.data['error-codes']?.join(', ') || 'Unknown error'; | |
| throw new BadRequestException(`Turnstile verification failed: ${errorCodes}`); | |
| } | |
| return true; | |
| try { | |
| const response = await axios.post<TurnstileVerifyResponse>(this.verifyUrl, formData.toString(), { | |
| headers: { | |
| 'Content-Type': 'application/x-www-form-urlencoded', | |
| }, | |
| }); | |
| if (!response.data.success) { | |
| const errorCodes = response.data['error-codes']?.join(', ') || 'Unknown error'; | |
| throw new BadRequestException(`Turnstile verification failed: ${errorCodes}`); | |
| } | |
| return true; | |
| } catch (error) { | |
| if (axios.isAxiosError(error)) { | |
| const status = error.response?.status; | |
| const message = | |
| (Array.isArray((error.response?.data as any)?.['error-codes']) && | |
| ((error.response?.data as any)['error-codes'] as string[]).join(', ')) || | |
| error.message || | |
| 'Unknown network or Cloudflare error'; | |
| throw new BadRequestException( | |
| `Turnstile verification failed due to network or Cloudflare error${status ? ` (status ${status})` : ''}: ${message}`, | |
| ); | |
| } | |
| throw error; | |
| } |
|
|
||
| if (!secretKey) { | ||
| console.warn('Turnstile secret key is not configured. Skipping verification.'); |
There was a problem hiding this comment.
Returning true when the secret key is not configured creates a security bypass in production. If TURNSTILE_SECRET_KEY is missing in production, the bot protection is completely disabled without failing loudly. Consider checking the environment (production vs development) and only allow bypassing in development/test environments, or throw an error in production.
| if (!secretKey) { | |
| console.warn('Turnstile secret key is not configured. Skipping verification.'); | |
| const nodeEnv = process.env.NODE_ENV || 'development'; | |
| if (!secretKey) { | |
| if (nodeEnv === 'production') { | |
| throw new Error('Turnstile secret key is not configured in production. Please set the TURNSTILE_SECRET_KEY environment variable.'); | |
| } | |
| console.warn('Turnstile secret key is not configured. Skipping verification (non-production environment).'); |
| role: UserRoleEnum; | ||
|
|
||
| @ApiProperty({ description: 'Cloudflare Turnstile token for bot protection' }) | ||
| @IsOptional() |
There was a problem hiding this comment.
The turnstileToken field is marked as @IsOptional(), but the service throws an error when the token is missing (line 24 in turnstile.service.ts throws BadRequestException if !token). This creates inconsistent validation - the DTO says optional, but the service requires it. Either make the field required in the DTO by removing @IsOptional(), or handle the optional case properly in the service.
| @IsOptional() | |
| @IsNotEmpty() |
| @Body() inviteUserData: InviteUserInCompanyAndConnectionGroupDto, | ||
| ) { | ||
| const { email, groupId, role, turnstileToken } = inviteUserData; | ||
| await this.turnstileService.verifyToken(turnstileToken); |
There was a problem hiding this comment.
The turnstile verification logic is not covered by tests. The test files in backend/test/ava-tests that exercise the /company/user/:companyId endpoint (company-info-e2e.test.ts) don't include turnstileToken in their request bodies. Since this is now a required field for bot protection, tests should be updated to include turnstile token validation scenarios.
No description provided.